Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for serving multiple apps with index with Bokeh and/ or Panel Server #1

Merged
merged 16 commits into from
Jun 28, 2021

Conversation

MarcSkovMadsen
Copy link

Implements: ideonate/cdsdashboards#65

This add support for

  • serving multiple apps. Either as a list of paths or as a glob
  • serving using Bokeh and/ or Panel

I walk through the new functionality in this video https://youtu.be/pVjj2vMwSIs. Check it out.

FYI. @danlester

Copy link
Member

@danlester danlester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for all of this work. Your insight into Panel and Bokeh is really important!

I just have a couple of ideas about this code. I've added a note inline suggesting we do the INDEX_HTML imports only when needed. This is mainly to save breaking if the installed components aren't quite right.

Related, it might make sense only to do the panel/bokeh imports when needed if possible. So a bokeh user doesn't have to have panel installed at all.

If we're going down that route, maybe panel (and possibly bokeh) should be removed from the requirements.txt - it's up to the user to manufacture their own environment. And installing panel can certainly be optional for them.

If we're going to keep requirements.txt then we definitely need a >= version number because I'm sure a lot of the components are fairly recent.

In the other similar commands I generally leave out the requirements for things like panel/voila. The reason is then that bokeh-root-cmd and friends can be installed as part of jhsingle-native-proxy (known as cdsdashboards-singleuser in conda), and still be relatively lightweight but have everything they will need for ContainDS Dashboards. Setting up the rest of the conda env is then down to them as normal. The idea is that if they want panel they will install it, same with voila, and that is something they should know how to do. But guessing that 'bokeh-root-cmd' is needed is not something they will know in general - so that's why I think it is good for all of those small components to be installed even if they are not going to be used.

By the way, please note that the glob doesn't work from cdsdashboards because it is your shell that is expanding the glob when running locally. If you put quotes around the "*.ipynb" you'll see it doesn't pick up the files.

Thanks again - I hope these comments are useful! Please let me know what you think.


print("Fetching Bokeh script or folder {}".format(app_py_path))
_BOKEH_INDEX_HTML = str(pathlib.Path(bokeh.server.views.__file__).parent / "app_index.html")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please could these HTML extractions happen inside the Bokeh/PanelServer classes?

So maybe there is a get_index_html() static method that performs this operation (and can 'cache' it in a static property too).

Of course this would save performing the operation when it's not needed. Partly, I suggest this for speed, but more importantly in case something goes wrong - especially if a bokeh user doesn't quite have the right panel installed (or has none at all), so the import breaks (when it wasn't actually needed anyway!)

I also have a related note about imports and requirements which I'll add below.

@MarcSkovMadsen
Copy link
Author

I think this all makes sense. Will look at it. Probably on the weekends.

@danlester
Copy link
Member

@MarcSkovMadsen I had a chance to work on some of this - hope you don't mind.

I have refactored so Panel imports happen only when needed.

And also tried to distinguish between "serving a folder containing lots of .py/.ipynb files" and "serving an app which is a main.py/main.ipynb in a folder".

e315b0c

I think it all makes sense, but your knowledge of Panel/Bokeh would be really helpful of course!

@danlester danlester merged commit dd2253e into ideonate:master Jun 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants